uefi-raw: Add binding for EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL #1658
Conversation
|
No idea what the CI is complaining about, tbh. I'm still quite unsure about how we should handle |
seijikun
force-pushed
the
mr-pciroot
branch
2 times, most recently
from
7357644 to
4adcc5c
Compare
Currently just a couple minor clippy things, add
Yep, when in doubt let's go with |
seijikun
force-pushed
the
mr-pciroot
branch
2 times, most recently
from
397aafc to
4dd2a16
Compare
|
Aight, when in doubt, This is the error I don't really understand: |
|
The error reporting of the |
|
Ah sorry I missed that error. It's because you're using a Rust enum, which is not quite compatible with a C enum (because it's undefined behavior to create a Rust enum from an invalid value). This can be fixed by using the (I put up #1660 to improve this error message.) |
seijikun
force-pushed
the
mr-pciroot
branch
3 times, most recently
from
6fd55f5 to
45b4059
Compare
Oof, that's embarrassing, the last merge request was only a few weeks ago and I've already forgotten about it. I needed some iterations for the addressing stuff ( |
|
|
||
| #[derive(Debug)] | ||
| #[repr(C)] | ||
| pub struct PciRootBridgeIoAccess<TAddr> { |
There was a problem hiding this comment.
The uefi-raw crate doesn't normally use generics. I think in the spec the address is treated as u64 in all cases, even though for for pci specifically the u64 is broken down into subfields, so let's do that and leave higher-level abstractions to the uefi crate.
There was a problem hiding this comment.
Yes, the interface itself in the spec accepts a u64, because they use the same struct for pci, memory and io.
But the pci one is supposed to be filled by something like this:
#define EFI_PCI_ADDRESS(bus, dev, func, reg) \
(UINT64) ( \
(((UINTN) bus) << 24) | \
(((UINTN) dev) << 16) | \
(((UINTN) func) << 8) | \
(((UINTN) (reg)) < 256 ? ((UINTN) (reg)) : (UINT64) (LShiftU64 ((UINT64) (reg), 32))))My thought process was: If they could have used templates/generics in their interface, they would have done it there.
At least I would like to have the PCI addressing somehow as part of the unsafe API, because otherwise you can't use it without reading the UEFI spec. Do you have an alternative suggestion? The only alternative I was able to come up with, was a union and that's IMHO ugly to use.
There was a problem hiding this comment.
In order to get rid of the generics, we could:
impl Into<u64> for PciIoAddress {
fn into(self) -> u64 {
unsafe { core::mem::transmute(self) }
}
}And then use it like so:
let addr = PciIoAddress::new(1, 3, 3).with_register(7);
(raw_proto.pci.read)(
raw_proto,
PciRootBridgeIoProtocolWidth::UINT32,
addr.into(), // API takes u64, we convert PciIoAddress to it
1,
&mut raw as *mut u32 as *mut c_void
).to_result()?;EDIT: Just refactored it that way
seijikun
force-pushed
the
mr-pciroot
branch
3 times, most recently
from
6901e11 to
7814063
Compare
Checklist